-
Notifications
You must be signed in to change notification settings - Fork 178
Avoid errors due to too large range bounds #6201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
One CI test job is failing, "Validate release scripts". How is this related to the current pull request? Is this problem something that should be addressed in the current pull request, or is this a separate issue that should be fixed first, and then we run the CI tests again? |
|
Yes, I'm not sure how that Python code got merged in, but it's obviously nothing to do with your PR. |
|
I'm not saying we shouldn't merge this, but I made another PR adding 'large ranges' to GAP ( #6202 ), which also seems to fix this problem. Of course, we might decide that's too big a change. |
|
@ChrisJefferson thanks. |
|
I'm really not sure why your PR is failing, because it looks fine to me when I run 'black' on the python files on my computer. Let's see if anyone else has a suggestion. |
|
@ChrisJefferson Concerning the formatting problem: Your pull request #6202 shows the same failure. |
|
Ignore the formatting failures. My guess is: Anyway: you don't need to worry about this. Regarding large ranges: I don't think we'll have them soon, so we definitely don't want to wait for that, and need a workaround first. |
So this pull request should be rebased (as soon as #6204 is merged), merged, and backported? |
and make sure that the runtime is short (this is of course cheating, without the "hint" for `GlobalMersenneTwister` the runtime will in general be much longer)
4135553 to
978c0a6
Compare
|
Yes (although the |
resolves #6200
Hopefully this is just a preliminary fix.
The change can be reverted as soon as large integers are allowed as bounds and length of ranges in GAP.